Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[remote/downloader] Add incompatible flag for sending a list of headers as qualifier #16312

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Sep 20, 2022

No description provided.

@Yannic Yannic requested a review from a team as a code owner September 20, 2022 20:46
@Yannic
Copy link
Contributor Author

Yannic commented Sep 20, 2022

@tjgq PTAL

Ideally, I'd like to flip this before Bazel 6.0.0 to limit who depends on the old behavior. Not sure how feasible that is though.

@ShreeM01 ShreeM01 added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Sep 21, 2022
@tjgq
Copy link
Contributor

tjgq commented Sep 22, 2022

@tjgq PTAL

Ideally, I'd like to flip this before Bazel 6.0.0 to limit who depends on the old behavior. Not sure how feasible that is though.

AIUI the 6.0.0 branch cut has been postponed to Oct 10 (#16159 (comment)), so we still have some time. I agree that we should flip it; the sooner the better, so that our nightly downstream pipeline can catch anything that breaks.

@@ -586,6 +586,15 @@ public RemoteOutputsStrategyConverter() {
+ "`all` to print always.")
public ExecutionMessagePrintMode remotePrintExecutionMessages;

@Option(
name = "incompatible_remote_downloader_send_all_headers",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you planning for this flag to eventually also affect HttpDownloader? If so, I'm wondering whether we should put it in a different Options class (not sure which one, though...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. This flag is specific to GrpcRemoteDownloader and the remote assets API for downloading.

Copy link
Contributor Author

@Yannic Yannic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL

aiuto pushed a commit to aiuto/bazel that referenced this pull request Oct 12, 2022
…rs as qualifier

Closes bazelbuild#16312.

PiperOrigin-RevId: 477770765
Change-Id: I31a2eac5b93c184dee7580a058b9b9fb10ddd9ff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants